-
Notifications
You must be signed in to change notification settings - Fork 194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Transition main navigation bar to new design system. #8381
Transition main navigation bar to new design system. #8381
Conversation
2ff720a
to
f8dd84a
Compare
f8dd84a
to
b28ddfe
Compare
@@ -9,47 +9,43 @@ | |||
navbar-default | |||
navbar-inverse | |||
navbar-static-top | |||
<% if !t('admin.whats_new.show_banner') && admin_template %>add-bottom-margin<% end %> | |||
<% if environment_style %>environment-indicator<% end %>" role="banner"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove the <header>
element at this stage so that we do not nest one of these elements within another, which could affect the way it renders.
I don't think it matters that this will also remove the navigation from header because we are also going to have to adopt a slightly different structure to the existing one when we use the layout_header
component, so that the navigation is not structurally part of the header but is a separate <nav>
element (but that is part of the next piece of work).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidtrussler
As discussed I have removed the header tag and update the design view. Now the view will look like below and it will get fixed once sub-navigation will get implemented.
<%= render partial: "shared/header" %> | ||
<% else %> | ||
<%= render partial: "shared/legacy_header" %> | ||
<% end %> | ||
</div> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will need to be restructured a bit like below so that when using the new header we don't nest it in the classes we don't need (legacy-whitehall
, navbar-wrapper
etc.)
<% if show_new_header? %>
<%= render partial: "shared/header" %>
<% else %>
<div class="legacy-whitehall">
<div class="environment-<%= environment %> navbar-wrapper">
<%= render partial: "shared/legacy_header" %>
</div>
</div>
<% end %>
b28ddfe
to
71be6c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes made in regard to the comments I left look good to me. 👍
92073b5
to
384cf44
Compare
384cf44
to
1ee85d7
Compare
Hi @nnagewad ! All users tab |
This commit includes below changes: -Duplicated existing header partial view to legacy. -Created new header partial with design system logic for main navigation. -Added new method in base controller to handle toggle between legacy and new header views. -Updated admin and design_system views with new header partial view render. -Added view tests to BaseController to verify new header renders for design system permissions.
Updates to use the `NewDocumentController` instead of the `DashboardController`, since the `DashboardController` has a dependency on a signed-in user. Moves the login line from setup to each test, since different tests have different requirements.
This commit removes govuk grid div styles which seems not required.
7799d79
to
37e8e6d
Compare
Nice! The changes look great! |
This commit includes: -Update main navigation to show active tabs highlighted. -Rename the sub-nav helper to header_helper to keep generic. -Added test to base controller.
37e8e6d
to
8d8a9c5
Compare
Latest changes LGTM. |
This PR transits main navigation bar to design system.
It does the following:
Screen shots:
Development env:
Integration env:
Production env:
Trello:
https://trello.com/c/UFcEeugT/560-transition-main-navigation-bar-to-new-design-system